Update interpolate action to output HEALPix_nested directly #50

tweska · 2025-01-20T10:58:01Z

This change removes the need to use the renumber-healpix action to interpolate to a HEALPix_nested. You can now do it in the interpolate action directly.

Draft because I think we might want to consider adding a few more things:

Extra 1024 test, I think the data needed already exists so we might as well.
Tests in the test/action/interpolate directory which will remain after we remove renumber-healpix
Some sort of warning message in the renumber-healpix, alerting users of this change.

src/multio/action/interpolate/Interpolate.cc

pmaciel · 2025-01-20T15:37:43Z

tests/multio/action/interpolate-healpix-nested/reduced_gg_to_HEALPix_1024_nested_original.yaml

+          caching: false
+      - type: metadata-mapping
+        mapping: '{~}/mapping_1024.yaml'
+      - type: renumber-healpix


I would remove the renumbering option altogether, as well as associated code -- this is legacy and not needed.

Removing renumber HEALPix requires a lot of modifications in climateDT. RAPS, multIO plans, and moreover there is still the FESOM interpolation (interpolation from matrix) pending question (who will remap the matrices?). My understanding is that now we are in a very busy phase and it is better to not touch for the moment.

pmaciel · 2025-01-20T15:38:16Z

tests/multio/action/interpolate-healpix-nested/reduced_gg_to_HEALPix_32_nested_original.yaml

+          caching: false
+      - type: metadata-mapping
+        mapping: '{~}/mapping_32.yaml'
+      - type: renumber-healpix


Again I'd remove all cases/code refering to renumbering

pmaciel

Excellent -- though I'd remove teh renumebering functionality/tests

FussyDuck · 2025-01-21T15:01:28Z

All committers have signed the CLA.

tweska · 2025-01-21T15:08:52Z

Added a test test with reference data generated without the changes in the interpolate action. The output is exactly the same as before, even if you use the original plan. As requested by @stefaniereuter

The commit from @MircoValentiniECMWF is a tool which lets you convert the matrices needed for fesom, this is work in progress. We still need to test it and possibly make a few changes.

MircoValentiniECMWF · 2025-01-21T22:06:39Z

some more points:

It will be nice to add to this script "src/multio/action/interpolate/generate_fesom_caches.sh" the management of the new command line flag to reorder the matrix ( or to create two scripts generate_fesom_cache_ring.sh generate_fesom_cache_nested.sh)
@dsarmany what shall we do with the HEALPix.* class? now I just copied the files in the interpolate action, but we have a copy in the renumber HEALPix action. Maybe we should put this calss in utils? or is it ok to have a duplication since renumberHELAPix should be removed in the future?
We need to update the interpolate action also for the "matrix" interpolation method. Same modification we made for the other method we need to inject the ordering convention (that has to be in the matrix name.)
It will be nice to have a test for the FESOM workflow. csv -> mat and then using the mat in the interpolate action to do an interpolation from FESOM grid to HEALPix nested directly

pmaciel · 2025-01-22T14:01:28Z

some more points:

1. It will be nice to add to this script "src/multio/action/interpolate/generate_fesom_caches.sh" the management of the new command line flag to reorder the matrix ( or to create two scripts generate_fesom_cache_ring.sh generate_fesom_cache_nested.sh)

2. @dsarmany what shall we do with the HEALPix.* class? now I just copied the files in the interpolate action, but we have a copy in the renumber HEALPix action. Maybe we should put this calss in utils? or is it ok to have a duplication since renumberHELAPix should be removed in the future?

3. We need to update the interpolate action also for the "matrix" interpolation method. Same modification we made for the other method we need to inject the ordering convention (that has to be in the matrix name.)

4. It will be nice to have a test for the FESOM workflow. csv -> mat and then using the mat in the interpolate action to do an interpolation from FESOM grid to HEALPix nested directly

For one of the points above, you can also modify the mir-codec-to-weight-matrix tool to renumber the input and/or output indices directly.

dsarmany · 2025-01-22T15:41:55Z

@dsarmany what shall we do with the HEALPix.* class? now I just copied the files in the interpolate action, but we have a copy in the renumber HEALPix action. Maybe we should put this calss in utils? or is it ok to have a duplication since renumberHELAPix should be removed in the future?

I think it is okay to keep the duplicates, given that we want to get rid of the whole action anyway. I am happy to give climate DT a bit of a breathing space right now. But at some point, and still sooner rather than later, I agree with @pmaciel that we should just simply remove this action and force the climate-DT plans to be updated.

github-actions · 2025-02-10T17:36:57Z

Caution

This pull request contains changes to GitHub workflows!
Proceed with caution and if not sure, contact your GitHub admin.

github-actions · 2025-02-11T08:51:54Z

Caution

This pull request contains changes to GitHub workflows!
Proceed with caution and if not sure, contact your GitHub admin.

…x action

… cache

github-actions · 2025-02-11T09:02:44Z

Caution

This pull request contains changes to GitHub workflows!
Proceed with caution and if not sure, contact your GitHub admin.

github-actions · 2025-02-11T10:53:52Z

Caution

This pull request contains changes to GitHub workflows!
Proceed with caution and if not sure, contact your GitHub admin.

github-actions bot added the contributor label Jan 20, 2025

pgeier requested review from MircoValentiniECMWF, dsarmany and pmaciel January 20, 2025 15:24

pmaciel reviewed Jan 20, 2025

View reviewed changes

src/multio/action/interpolate/Interpolate.cc Outdated Show resolved Hide resolved

pmaciel reviewed Jan 20, 2025

View reviewed changes

pmaciel previously approved these changes Jan 20, 2025

View reviewed changes

tweska dismissed pmaciel’s stale review via 49b19c6 January 20, 2025 18:34

pmaciel previously approved these changes Jan 22, 2025

View reviewed changes

tweska dismissed pmaciel’s stale review via a73e785 February 10, 2025 17:36

tweska requested a review from stefaniereuter February 10, 2025 17:37

tweska and others added 11 commits February 11, 2025 08:52

Add test for existing HEALPix_nested conversion using renumber-healpi…

90568f0

…x action

Update interpolate action to accept HEALPix_nested grid kind

5037f13

Add tests to compare old and new method to produce HEALPix_nested

4c8bd2c

Add comparison test for nested HEALPix 1024 interpolation

0eb5fa1

Fix dependencies in interpolate HEALPix nested tests

0d05f9a

WIP: added functionality to remap fesom matrices when we generate the…

ecdfaa1

… cache

Add HEALPix nested test to compare against reference data

6b88cea

Set the correct ordering convention for matrix interpolation

dd98167

Update fesom to mir cache generator options

12184b4

Add fesom to mir cache generator tests

2cb68ff

Make set_tests_properties call conditional

5f8afab

Add warning message in renumber-healpix action

638ab95

tweska force-pushed the healpix-nested branch from f0c3c0e to 638ab95 Compare February 11, 2025 09:02

Update tool for batch generating mat files in nested ordering for fesom

95917c3

tweska marked this pull request as ready for review February 11, 2025 10:54

tweska closed this Feb 13, 2025

tweska mentioned this pull request Feb 13, 2025

Support HEALPix_nested in interpolate action #56

Merged

Update interpolate action to output HEALPix_nested directly #50

Update interpolate action to output HEALPix_nested directly #50

Uh oh!

Conversation

tweska commented Jan 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

pmaciel Jan 20, 2025

Choose a reason for hiding this comment

Uh oh!

MircoValentiniECMWF Jan 20, 2025

Choose a reason for hiding this comment

Uh oh!

pmaciel Jan 20, 2025

Choose a reason for hiding this comment

Uh oh!

pmaciel left a comment

Choose a reason for hiding this comment

Uh oh!

FussyDuck commented Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tweska commented Jan 21, 2025

Uh oh!

MircoValentiniECMWF commented Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pmaciel commented Jan 22, 2025

Uh oh!

dsarmany commented Jan 22, 2025

Uh oh!

github-actions bot commented Feb 10, 2025

Uh oh!

github-actions bot commented Feb 11, 2025

Uh oh!

github-actions bot commented Feb 11, 2025

Uh oh!

github-actions bot commented Feb 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tweska commented Jan 20, 2025 •

edited

Loading

FussyDuck commented Jan 21, 2025 •

edited

Loading

MircoValentiniECMWF commented Jan 21, 2025 •

edited

Loading